Skip to content

symbol_names: treat ReifyShim like VtableShim. #70003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 22, 2020
Merged

symbol_names: treat ReifyShim like VtableShim. #70003

merged 1 commit into from
Mar 22, 2020

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 14, 2020

Without this, the #[track_caller] tests don't pass with -Zsymbol-mangling-version=v0, because there is a symbol name collision between the ReifyShim and the original definition.

cc @anp

@rust-highfive
Copy link
Contributor

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 14, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 14, 2020

r? @nikomatsakis cc @michaelwoerister

nikomatsakis
nikomatsakis approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to update any user-facing documentation?

In particular, for the v0 symbol names, we had an RFC, have we documented those symbol names anywhere? I think we should -- and specifically I think we should as part of the rustc book, I guess? But maybe the shim:... stuff is sort of "impl defined"..?

@eddyb
Copy link
Member Author

eddyb commented Mar 19, 2020 •
edited
Loading

But maybe the shim:... stuff is sort of "impl defined"..?

Yeah, and we reserved pretty much all of A-Z for "special namespaces".
C and S are the main ones recognized by demanglers, IIRC, as "closure" and "shim".

@nikomatsakis
Copy link
Contributor

@bors r+

OK -- though I still think we should have the docs in some place (presuming that we don't), but that's not a reason to block this PR.

@bors
Copy link
Collaborator

bors commented Mar 19, 2020

📌 Commit d01622ccc46f5daf75798c58bdd9330a75cbf9ab has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2020
@bors
Copy link
Collaborator

bors commented Mar 20, 2020

☔ The latest upstream changes (presumably #70174) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 20, 2020
@eddyb eddyb force-pushed the symbol-mangling-reify-shims branch from d01622c to b7156e8 Compare March 21, 2020 03:25
@eddyb
Copy link
Member Author

eddyb commented Mar 21, 2020

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Mar 21, 2020

📌 Commit b7156e8890ed11a71b6bd1d0041bdb641e5dd86f has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 21, 2020
@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 21, 2020
@eddyb eddyb force-pushed the symbol-mangling-reify-shims branch from b7156e8 to 14e0aad Compare March 21, 2020 12:52
@eddyb
Copy link
Member Author

eddyb commented Mar 21, 2020

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Mar 21, 2020

📌 Commit 14e0aad has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 21, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#70003 - eddyb:symbol-mangling-reify-shims, …
638857f 
…r=nikomatsakis

symbol_names: treat ReifyShim like VtableShim.

Without this, the `#[track_caller]` tests don't pass with `-Zsymbol-mangling-version=v0`, because there is a symbol name collision between the `ReifyShim` and the original definition.

cc @anp
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 21, 2020
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 21, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2020
@bors
Auto merge of rust-lang#70246 - Dylan-DPC:rollup-vt9wex2, r=Dylan-DPC
c6b172f 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#70003 (symbol_names: treat ReifyShim like VtableShim.)
 - rust-lang#70051 (Allow `hir().find` to return `None`)
 - rust-lang#70126 (Fix ICE caused by truncating a negative ZST enum discriminant)
 - rust-lang#70197 (For issue 53957: revise unit test to focus on underlying bug of 23076.)
 - rust-lang#70215 (ast: Compress `AttrId` from `usize` to `u32`)
 - rust-lang#70218 (Fix deprecated Error.description() usage in docs)
 - rust-lang#70228 (Remove CARGO_BUILD_TARGET from bootstrap.py)
 - rust-lang#70231 (Add explanation message for E0224)
 - rust-lang#70232 (Tweak wording for std::io::Read::read function)
 - rust-lang#70238 (Add a test for out-of-line module passed through a proc macro)

Failed merges:

r? @ghost
@bors bors merged commit 834ed36 into rust-lang:master Mar 22, 2020
@eddyb eddyb deleted the symbol-mangling-reify-shims branch March 22, 2020 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eddyb @rust-highfive @nikomatsakis @bors @estebank